feat: workflow v0.14.1 — security-audit + provenance + GitLab auth#414
Conversation
New cmd/wfctl/build_security_audit.go implements runBuildSecurityAudit with six checks: hardened flag, dockerfile sbom/provenance, registry retention, plugins lockfile, auth env vars, and local-env hardening override (NOTE only). Wired as `wfctl build audit`. --strict exits 1 when any WARN is present. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When ci.build.security.hardened=true, buildWithDockerfile appends --provenance=mode=max and --sbom=true to the docker build invocation. Emits a warning when DOCKER_BUILDKIT!=1 to flag that BuildKit is required for provenance to actually work. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace the stub with a real implementation: Login uses gitlab-ci-token in CI context (CI_JOB_TOKEN), oauth2 otherwise. Push delegates to docker push. Prune calls the GitLab API to delete tags beyond retention.keep_latest. Add 7 dry-run tests mirroring DO/GHCR patterns. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds v0.14.1 follow-up release features focused on supply-chain hardening and registry support, including a new build security audit command, BuildKit provenance/SBOM flags, and a GitLab Container Registry provider implementation.
Changes:
- Added
wfctl build audit(with--strict) to scan configs for supply-chain security issues. - Updated
wfctl build imageto append BuildKit provenance + SBOM flags whenci.build.security.hardened=true. - Implemented GitLab registry provider
Login/Push/Pruneand added corresponding tests; updated CHANGELOG for v0.14.1.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
cmd/wfctl/build_security_audit.go |
Implements the new wfctl build audit command and the six audit checks + strict mode. |
cmd/wfctl/build_security_audit_test.go |
Adds unit tests covering audit findings and --strict exit behavior. |
cmd/wfctl/build_image.go |
Appends provenance/SBOM flags to docker builds when hardened is enabled. |
cmd/wfctl/build_image_test.go |
Tests for provenance/SBOM arg behavior under hardened vs non-hardened. |
cmd/wfctl/build.go |
Wires audit into wfctl build subcommand dispatch. |
plugins/registry-gitlab/plugin.go |
Replaces stub with GitLab registry provider implementation (Login/Push/Prune). |
plugins/registry-gitlab/plugin_test.go |
Adds tests for GitLab provider dry-run output and auth error handling. |
CHANGELOG.md |
Documents v0.14.1 additions for audit, provenance, and GitLab registry support. |
| tagsURL := fmt.Sprintf("%s/api/v4/projects/%s/registry/repositories/%d/tags", baseURL, encodedProject, repo.ID) | ||
| tags, err := glGetJSON[[]glRepoTag](ctx, token, tagsURL) | ||
| if err != nil { | ||
| return fmt.Errorf("list tags for repo %d: %w", repo.ID, err) |
There was a problem hiding this comment.
GitLab's .../tags endpoint is paginated. Fetching only the first page means keepLatest will be applied to a partial tag set, so older tags beyond the first page will never be pruned. Consider adding per_page and iterating through all pages before sorting/applying retention, or alternatively repeatedly delete from subsequent pages until the tag count is within keepLatest.
| if err != nil { | ||
| return zero, err | ||
| } | ||
| req.Header.Set("PRIVATE-TOKEN", token) | ||
|
|
There was a problem hiding this comment.
All GitLab API calls here send the token as PRIVATE-TOKEN, but resolveCredentials can return a CI_JOB_TOKEN. GitLab expects JOB-TOKEN for job tokens (and PRIVATE-TOKEN/Authorization for PAT/OAuth). As-is, Prune is likely to fail in CI contexts where only CI_JOB_TOKEN is available. Consider returning an auth mode/header from resolveCredentials (or a helper that applies the correct headers) and using it for all API requests.
| if err != nil { | ||
| return err | ||
| } | ||
| req.Header.Set("PRIVATE-TOKEN", token) | ||
| resp, err := http.DefaultClient.Do(req) |
There was a problem hiding this comment.
Tag deletions also always authenticate using PRIVATE-TOKEN. If the token is coming from CI_JOB_TOKEN, GitLab requires JOB-TOKEN instead, so deletions will likely return 401/403 in GitLab CI. Use the correct header depending on credential type (job token vs PAT/OAuth), ideally via a shared helper to keep GET/DELETE consistent.
| case "audit", "--security-audit": | ||
| return runBuildSecurityAudit(rest) |
There was a problem hiding this comment.
wfctl build --security-audit cannot work with the current dispatch logic: arguments starting with '-' are treated as flags (see isFlag), so this case is never reached, and the main FlagSet doesn't define a --security-audit flag. Invoking wfctl build --security-audit will fail with an unknown-flag parse error instead of running the audit. Consider either adding a real --security-audit boolean flag to the main wfctl build FlagSet (and invoking the audit before orchestration), or removing the --security-audit alias here and documenting wfctl build audit as the supported entrypoint.
|
|
||
| // T33: BuildKit provenance attestation when hardened=true. | ||
| if hardened { | ||
| if os.Getenv("DOCKER_BUILDKIT") != "1" { |
There was a problem hiding this comment.
The BuildKit warning is based on the current process environment, but the actual docker command always forces DOCKER_BUILDKIT=1 via cmd.Env. As written, this can emit a warning in non-dry-run executions even though the build will run with BuildKit enabled. Consider gating this warning to dry-run only, or basing it on whether the invoked command will have BuildKit enabled (e.g., the env you set on cmd).
| if os.Getenv("DOCKER_BUILDKIT") != "1" { | |
| if dryRun && os.Getenv("DOCKER_BUILDKIT") != "1" { |
| for _, f := range findings { | ||
| if f.Severity == "WARN" { | ||
| return fmt.Errorf("%d build security issue(s) found", len(findings)) | ||
| } | ||
| } |
There was a problem hiding this comment.
In --strict mode the returned error message uses len(findings) as the count, but findings can include NOTE-level entries as well as WARN. This can over-report the number of warning issues triggering the non-zero exit. Consider counting only Severity == "WARN" findings for the strict-mode error (and potentially wording the message as warnings).
| for _, f := range findings { | |
| if f.Severity == "WARN" { | |
| return fmt.Errorf("%d build security issue(s) found", len(findings)) | |
| } | |
| } | |
| warnCount := 0 | |
| for _, f := range findings { | |
| if f.Severity == "WARN" { | |
| warnCount++ | |
| } | |
| } | |
| if warnCount > 0 { | |
| return fmt.Errorf("%d build security warning(s) found", warnCount) | |
| } |
| encodedProject := url.PathEscape(projectPath) | ||
| baseURL := "https://gitlab.com" | ||
|
|
There was a problem hiding this comment.
The GitLab API base URL is hard-coded to https://gitlab.com. This will break Prune for self-managed GitLab instances (where the API host is different), even if the registry path points at a different domain. Consider deriving the API base from configuration (e.g., from cfg.Registry.Path or an explicit api_base_url setting) so the provider works outside gitlab.com.
| // List registry repositories for the project. | ||
| repoURL := fmt.Sprintf("%s/api/v4/projects/%s/registry/repositories", baseURL, encodedProject) | ||
| repos, err := glGetJSON[[]glRepository](ctx, token, repoURL) | ||
| if err != nil { |
There was a problem hiding this comment.
GitLab's projects/.../registry/repositories endpoint is paginated. Fetching it once without per_page and without following pagination headers can miss repositories for larger projects, causing Prune to leave tags untouched. Consider requesting a larger per_page and/or iterating pages using X-Next-Page/Link headers.
…audit (T34) Extends the config-level audit with per-target linting: - Calls builder.SecurityLint() for each typed target (go/nodejs/custom) - For method:dockerfile containers: scans Dockerfile for USER root (critical), missing USER (critical), FROM :latest (warn), ADD URL (warn), embedded secrets (critical), base image policy violations (warn) - Exit code: CRITICAL → always 1; --strict → 1 on any warn - 9 new tests covering each Dockerfile rule and exit-code semantics Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
…en header a) Add CIRegistry.APIBaseURL for self-managed GitLab instances; default to https://gitlab.com when unset. b/c) Replace glGetJSON with glPaginatedGet that follows X-Next-Page headers for both repository and tag listing endpoints. d) resolveCredentials now returns tokenType ("job"|"private"); use JOB-TOKEN header for CI_JOB_TOKEN, PRIVATE-TOKEN for PATs. AuthHeaderFor exported for testing. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… in strict mode e) Remove unreachable --security-audit alias from build dispatcher; only `wfctl build audit` is supported. f) --strict now counts only WARN findings (not NOTE) when deciding exit code. Error message reports warn count, not total finding count. Add test asserting NOTE-only findings don't trigger --strict exit 1. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The DOCKER_BUILDKIT warning is misleading in live mode because cmd.Env already forces DOCKER_BUILDKIT=1 on the docker subprocess. Only emit the warning during dry-run when the env truly won't be set. Add test asserting the warning appears in dry-run and doesn't cause false positives in live builds. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
| hasUser := false | ||
| scanner := bufio.NewScanner(strings.NewReader(string(data))) | ||
| lineNum := 0 | ||
| for scanner.Scan() { | ||
| lineNum++ | ||
| line := strings.TrimSpace(scanner.Text()) | ||
|
|
||
| if reUserRoot.MatchString(line) { | ||
| addDF("CRITICAL", "USER root detected — container will run as root", lineNum) | ||
| } | ||
| if reUserAny.MatchString(line) { | ||
| hasUser = true | ||
| } | ||
| if reFromLatest.MatchString(line) { | ||
| addDF("WARN", "FROM uses :latest tag — pin to a digest or explicit version for reproducibility", lineNum) | ||
| } | ||
| if reAddURL.MatchString(line) { | ||
| addDF("WARN", "ADD with URL is untrusted — use RUN curl/wget with checksum verification instead", lineNum) | ||
| } | ||
| if reEmbeddedSecret.MatchString(line) { | ||
| addDF("CRITICAL", fmt.Sprintf("possible embedded secret in line %d — use BuildKit secrets (--secret) instead", lineNum), lineNum) | ||
| } | ||
|
|
||
| // Base image policy. | ||
| if len(allowPrefixes) > 0 && reFromImage.MatchString(line) { | ||
| m := reFromImage.FindStringSubmatch(line) | ||
| if len(m) > 1 && m[1] != "scratch" { | ||
| img := m[1] | ||
| if !matchesAnyPrefix(img, allowPrefixes) { | ||
| addDF("WARN", fmt.Sprintf("base image %q does not match allow_prefixes policy %v", img, allowPrefixes), lineNum) | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if !hasUser { | ||
| findings = append(findings, buildAuditFinding{ | ||
| Severity: "CRITICAL", | ||
| Check: checkName, | ||
| Message: "no USER directive found — container will run as root by default", | ||
| File: dfPath, | ||
| Line: 0, | ||
| }) | ||
| } |
There was a problem hiding this comment.
Dockerfile linting currently treats any USER directive anywhere in the file as sufficient. In multi-stage Dockerfiles, a USER in an earlier stage can cause a false negative if the final stage omits USER (defaulting back to root). Consider tracking USER per stage by resetting hasUser on each FROM, and only enforcing the USER rule (and USER root) on the final stage.
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
lintDockerfile doesn’t check scanner.Err() after the scan loop. If the Dockerfile contains very long lines (bufio.Scanner token limit) or an I/O error occurs, the scan can terminate early and findings may be incomplete without any signal. Add an if err := scanner.Err(); err != nil { ... } path (likely a WARN/CRITICAL finding) after the loop.
| } | |
| } | |
| if err := scanner.Err(); err != nil { | |
| addDF("WARN", fmt.Sprintf("failed to fully scan Dockerfile: %v", err), 0) | |
| } |
| // TestGitLabProvider_Prune_SelfManaged checks dry-run mentions the custom API base URL. | ||
| func TestGitLabProvider_Prune_SelfManaged(t *testing.T) { | ||
| p := registrygitlab.New() | ||
| reg := config.CIRegistry{ | ||
| Name: "self-managed", | ||
| Type: "gitlab", | ||
| Path: "registry.example.com/myorg/myproject", | ||
| APIBaseURL: "https://gitlab.example.com", | ||
| Auth: &config.CIRegistryAuth{Env: "GITLAB_TOKEN"}, | ||
| Retention: &config.CIRegistryRetention{KeepLatest: 3}, | ||
| } | ||
| t.Setenv("GITLAB_TOKEN", "glpat-selfmanaged-token") | ||
|
|
||
| var buf bytes.Buffer | ||
| ctx := registry.NewContext(t.Context(), &buf, true) | ||
| if err := p.Prune(ctx, registry.ProviderConfig{Registry: reg}); err != nil { | ||
| t.Fatalf("Prune dry-run self-managed: %v", err) | ||
| } | ||
|
|
||
| out := buf.String() | ||
| if !strings.Contains(out, "myorg/myproject") { | ||
| t.Errorf("expected project path in dry-run output, got: %q", out) | ||
| } |
There was a problem hiding this comment.
This test comment says the dry-run output mentions the custom API base URL, but the assertions only check for the project path (and the provider’s dry-run output doesn’t currently include APIBaseURL). Either update the comment to match what’s actually asserted, or extend the assertion/output to include the API base URL.
| func runBuildAuditChecks(cfgPath, workDir string) []buildAuditFinding { | ||
| cfg, err := config.LoadFromFile(cfgPath) | ||
| if err != nil { | ||
| return []buildAuditFinding{{Severity: "WARN", Check: "config", Message: fmt.Sprintf("failed to load config: %v", err)}} |
There was a problem hiding this comment.
runBuildAuditChecks converts config load failures into a WARN finding. As a result, wfctl build audit can exit 0 (when not --strict) even though the audit never ran. Consider returning an error from runBuildSecurityAudit when the config can't be loaded (or mark this finding as CRITICAL so it always forces a non-zero exit).
| return []buildAuditFinding{{Severity: "WARN", Check: "config", Message: fmt.Sprintf("failed to load config: %v", err)}} | |
| return []buildAuditFinding{{Severity: "CRITICAL", Check: "config", Message: fmt.Sprintf("failed to load config: %v", err)}} |
| return fmt.Sprintf("[%s] %s: %s", f.Severity, f.Check, f.Message) | ||
| } | ||
|
|
||
| // runBuildSecurityAudit implements `wfctl build audit` and `wfctl build --security-audit`. |
There was a problem hiding this comment.
The comment says this implements wfctl build --security-audit, but there’s no --security-audit flag wired into wfctl build in this PR (only the build audit subcommand is added). Either implement the flag and route it here, or update the comment to avoid advertising an unsupported CLI option.
| // runBuildSecurityAudit implements `wfctl build audit` and `wfctl build --security-audit`. | |
| // runBuildSecurityAudit implements `wfctl build audit`. |
Summary
Follow-up release addressing three features deferred from v0.14.0:
wfctl build audit(a944096): 6 supply-chain security checks +--strictflag. Checks hardening flag, dockerfile without SBOM/provenance, registries without retention, plugins without lockfile, unset env-var auth tokens, local environments that disable hardening.ci.build.security.hardened=true,wfctl build imagenow appends--provenance=mode=max --sbom=trueto the docker build invocation. Warns ifDOCKER_BUILDKIT=1is not set.plugins/registry-gitlabnow implements Login/Push/Prune. SupportsCI_JOB_TOKENauth for GitLab CI contexts and PAT-based auth elsewhere.CHANGELOG updated with
## v0.14.1section.Diff
cmd/wfctl/build_security_audit.go+ test — new audit commandcmd/wfctl/build.go— wiredwfctl build auditsubcommandcmd/wfctl/build_image.go+ test — provenance/sbom args when hardenedplugins/registry-gitlab/plugin.go+ test — GitLab CR providerTest plan
go test ./cmd/wfctl/... ./plugins/registry-gitlab/... -count=1— all green locally (21 new tests)golangci-lint run --timeout=10m— zero issues locallyRelated
docs/plans/2026-04-18-wfctl-build-deploy-design.mddocs/plans/2026-04-18-wfctl-build-deploy.md🤖 Generated with Claude Code